Phase 2 of rk_stratiform CCPPization: diagnostic schemes#234
Phase 2 of rk_stratiform CCPPization: diagnostic schemes#234jimmielin merged 13 commits intoESCOMP:developmentfrom
Conversation
|
Completed in 2639a45 |
nusbaume
left a comment
There was a problem hiding this comment.
Looks good @jimmielin! I did have some questions and suggestions, but hopefully nothing that will be too difficult to resolve (although if that is not the case please let me know). Thanks!
| call history_add_field('DLSED', 'tendency_of_cloud_liquid_water_mixing_ratio_wrt_moist_air_and_condensed_water_due_to_sedimentation', 'lev', 'avg', 'kg kg-1 s-1') | ||
| call history_add_field('HSED', 'tendency_of_dry_air_enthalpy_at_constant_pressure_due_to_sedimentation', 'lev', 'avg', 'J kg-1 s-1') | ||
| call history_add_field('PRECSED', 'stratiform_cloud_water_surface_flux_due_to_sedimentation', horiz_only, 'inst', 'm s-1') | ||
| call history_add_field('SNOWSED', 'lwe_cloud_ice_sedimentation_rate_at_surface_due_to_microphysics', horiz_only, 'inst', 'm s-1') |
There was a problem hiding this comment.
Is there a reason SNOWSED has a different long name compare to the PRECSED and RAINSED variables? For example I imagine one could do the following instead to better match the other two fields:
stratiform_lwe_cloud_ice_surface_flux_due_to_sedimentation
But maybe there is a reason to keep the long name as-is?
There was a problem hiding this comment.
Thanks for the question! SNOWSED had a previously-assigned standard name from pbuf_SNOW_SED so it looks different than the others maybe due to the passage of time. So maybe it can be updated to be up to the more recent guidelines / consistency with the current standard names?
There was a problem hiding this comment.
Thanks for the explanation @jimmielin! Given that SNOWSED appears to be the exact same physical quantity as PRECSED but just for ice I might vote to give the name I specified above. Of course if this is going to involve additional work elsewhere (e.g. in other physics schemes) then feel free to just make an issue for it and we can deal with it later. Thanks!
There was a problem hiding this comment.
Thanks @nusbaume, updated to stratiform_lwe_cloud_ice_surface_flux_due_to_sedimentation!
schemes/sima_diagnostics/cloud_particle_sedimentation_diagnostics.F90
Outdated
Show resolved
Hide resolved
nusbaume
left a comment
There was a problem hiding this comment.
Looks great to me now! I did leave one comment unresolved with regards to the SNOWSED variable but I am happy to just make an issue for it and deal with it later if that is easiest.
…x_due_to_sedimentation
cacraigucar
left a comment
There was a problem hiding this comment.
Note - I did not review the SDFs - I assume that they mimic what is being done in CAM
| @@ -0,0 +1,85 @@ | |||
| ! Diagnostics for RK stratiform - cloud particle sedimentation | |||
| ! Haipeng Lin, March 2025 | |||
There was a problem hiding this comment.
I'm questioning the need/wisdom of tagging the author/date of the new routines. I don't believe this has been done with other converted routines and git gives you the information. I know CAM did this for awhile, but everytime somebody edited the routine, they put their name/data/changes at the top of the routine and this became fairly large for some routines. I think we have moved away from this since we now have git tracking everything,
There was a problem hiding this comment.
Thanks @cacraigucar! I wonder about this as well and maybe we should loop others into this discussion.
I am happy to remove this for the diagnostic schemes since these are new files in atmospheric_physics, but for converted schemes (many of which had the header of name/data/changes you mentioned) where code was moved from CAM, Git won't have the history since it's not moved over from CAM and I've decided to copy over (or track down if needed) the original authors list for attribution purposes. Some of the history, particularly for CAM4, is only in SVN so Git won't show those either. I'm happy to discuss and go either way!
There was a problem hiding this comment.
I believe we agreed that these authorship comments should be removed
There was a problem hiding this comment.
Thanks, removed throughout!
| @@ -0,0 +1,85 @@ | |||
| ! Diagnostics for RK stratiform - cloud particle sedimentation | |||
| ! Haipeng Lin, March 2025 | |||
There was a problem hiding this comment.
I believe we agreed that these authorship comments should be removed
|
SIMA regression tests passed except FCAM4 changes (expected). There is one new RK test introduced for Derecho/GNU to test snapshots. A future test will be added in ESCOMP/CAM-SIMA#397 for testing history output baselines of physics schemes. |
Tag name (The PR title should also include the tag name): atmos_phys0_14_000 Originator(s): peverwhee List all `development` PR numbers included in this PR and the title of each: 1. Draft aerosol classes (#209) 2. Fix broken dme_adjust tests (#237) 3. Fix erroneous code in MUSICA during the CAM-SIMA run (#226) 4. Initial file reader object (#240) 5. Break up file i/o object into explicit interfaces (#248) 6. Update MUSICA library (#249) 7. Fix invalid horizontal dimensions (#251) 8. Add YSU orographic gravity wave drag scheme (#232) 9. Phase 2 of rk_stratiform CCPPization: diagnostic schemes (#234) 10. Update MUSICA suite (#252) 11. RRTMGP longwave fortran modules (#230) List all automated tests that failed, as well as an explanation for why they weren't fixed: n/a
Originator(s): @jimmielin
Description (include issue title and the keyword ['closes', 'fixes', 'resolves'] and issue number):
List all namelist files that were added or changed: N/A
List all files eliminated and why: N/A
List all files added and what they do:
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>)List all automated tests that failed, as well as an explanation for why they weren't fixed:
N/A
Is this an answer-changing PR? If so, is it a new physics package, algorithm change, tuning change, etc?
N/A - SIMA diagnostics only
If yes to the above question, describe how this code was validated with the new/modified features:
N/A